Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add LocalRuntime #5284

Merged
merged 110 commits into from
Feb 7, 2025
Merged

feat: Add LocalRuntime #5284

merged 110 commits into from
Feb 7, 2025

Conversation

xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Nov 26, 2024 •
edited by github-actions bot
Loading

This PR adds a new LocalRuntime implementation and renames EventStreamRuntime to LocalDockerRuntime for better clarity.

Changes

  • Add new LocalRuntime implementation that runs action_execution_server directly on the host machine
  • Rename EventStreamRuntime to LocalDockerRuntime for better clarity
  • Move runtime implementations to dedicated directories (local/ and docker/)
  • Update documentation to reflect runtime changes and add LocalRuntime description

Benefits

  • Provides a simpler setup for users who don't need container isolation
  • Clearer naming convention for runtime implementations
  • Better organized codebase with dedicated directories for each runtime type
  • Updated documentation to help users choose the right runtime for their needs

Issue fixed: https://github.com/orgs/All-Hands-AI/projects/1?pane=issue&itemId=82616729&issue=All-Hands-AI%7COpenHands%7C3903

To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:350e1bb-nikolaik   --name openhands-app-350e1bb   docker.all-hands.dev/all-hands-ai/openhands:350e1bb

@openhands-agent
feat: Add LocalRuntime and rename EventStreamRuntime to LocalDockerRu…
149f60a 
…ntime

- Add new LocalRuntime implementation that runs action_execution_server directly on host
- Rename EventStreamRuntime to LocalDockerRuntime for clarity
- Move runtime implementations to dedicated directories
- Update documentation to reflect runtime changes
@openhands-agent
test: Add LocalRuntime to test configuration
39911f4 
- Update imports to use LocalDockerRuntime and LocalRuntime
- Add LocalRuntime to get_runtime_classes()
- Update _close_test_runtime to handle LocalDockerRuntime
@xingyaoww xingyaoww mentioned this pull request Nov 26, 2024
Closed
2 tasks
@openhands-agent
fix: Fix mypy errors in LocalRuntime
1c9b6ca 
- Add proper type hints for server_process
- Fix stdout access safety
- Fix async/await type hints
- Improve error handling
enyst
enyst reviewed Nov 26, 2024
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@openhands-agent
Copy link
Contributor

New OpenHands update

@enyst
Copy link
Collaborator

enyst commented Nov 29, 2024

@openhands-agent This PR tests fail with

File "/home/runner/work/OpenHands/OpenHands/openhands/runtime/init.py", line 3, in
from openhands.runtime.impl.eventstream.eventstream_runtime import (

ERROR:root:<class 'ModuleNotFoundError'>: No module named 'openhands.runtime.impl.eventstream'

Fix this error. Keep your changes minimal.

@github-actions GitHub Actions
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

enyst
enyst reviewed Nov 29, 2024
View reviewed changes
enyst
enyst reviewed Nov 29, 2024
View reviewed changes
enyst
enyst reviewed Nov 29, 2024
View reviewed changes
@enyst
Copy link
Collaborator

enyst commented Nov 29, 2024

@openhands-agent We have done a lot to fix the original. This PR is doing these:

  • Add new LocalRuntime implementation that runs action_execution_server directly on the host machine
  • Rename EventStreamRuntime to DockerRuntime for better clarity
  • Move runtime implementations to dedicated directories (local/ and docker/)
  • Update documentation to reflect runtime changes and add LocalRuntime description.

The PR is mostly finished now. It has only one detail that went wrong: instead of the name LocalDockerRuntime, we now want the name DockerRuntime. Find all occurrences and fix them. Additionally, look also for the name EventStreamRuntime, and if you find any, update it to the new name, DockerRuntime.

@github-actions GitHub Actions
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@All-Hands-AI All-Hands-AI deleted a comment from openhands-agent Nov 29, 2024
@All-Hands-AI All-Hands-AI deleted a comment from github-actions bot Nov 29, 2024
@All-Hands-AI All-Hands-AI deleted a comment from github-actions bot Nov 29, 2024
@xingyaoww
Copy link
Collaborator Author

For some reason i cannot get these LocalRuntime tests to run in CI 😓 but they pass locally, and the test runs extremely slow in CI - maybe we could ask review for this PR as is, and add tests later? We probably should make this as some sort of highly experimental feature, and YMMV

image

@enyst
Copy link
Collaborator

enyst commented Feb 6, 2025 •
edited
Loading

@xingyaoww I agree. I fought them too, and yes, I can confirm they can work locally on Mac.

If you could take a look in conversations tab on github, it has some minor thoughts about errors/exceptions that I thought maybe we can discuss?

@scosenza
Copy link

scosenza commented Feb 7, 2025

I just wanted to thank you again for working on this PR! We've been regularly pulling this branch, and the local mode has been running great for us on Linux machines! 🙌

@xingyaoww
Copy link
Collaborator Author

xingyaoww commented Feb 7, 2025 •
edited
Loading

Thanks @enyst for the comments! I believe I should've addressed them -- LMK if you have any other question/concerns here. This is ready for another review :)

@xingyaoww xingyaoww changed the title feat: Add LocalRuntime and rename EventStreamRuntime to DockerRuntime feat: Add LocalRuntime Feb 7, 2025
enyst
enyst approved these changes Feb 7, 2025
View reviewed changes
Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this!

*makes a backup of the local machine

Let the fun begin 😂

@xingyaoww
Copy link
Collaborator Author

*makes a backup of the local machine

This actually makes actually think we should add this to the warning lol

@xingyaoww xingyaoww enabled auto-merge (squash) February 7, 2025 16:32
@xingyaoww xingyaoww merged commit 478b225 into main Feb 7, 2025
14 checks passed
@xingyaoww xingyaoww deleted the add-local-runtime branch February 7, 2025 16:35
chuckbutkus pushed a commit that referenced this pull request Feb 7, 2025
@xingyaoww @openhands-agent
@enyst @chuckbutkus
feat: Add LocalRuntime (#5284)
0b908d3 
Co-authored-by: openhands <[email protected]>
Co-authored-by: Engel Nyst <[email protected]>
@scosenza scosenza mentioned this pull request Feb 10, 2025
Open
@enyst enyst mentioned this pull request Feb 11, 2025
Closed
@czq99972
Copy link

@xingyaoww How should I start a service locally using the Local Runtime instead of Docker Runtime? What are the specific commands and steps for executing this?

@enyst
Copy link
Collaborator

enyst commented Feb 21, 2025

All I had to do to use it is to set the runtime type to local in config.toml (e.g. like the template shows here.

But best use is probably not local exactly, but on another machine than your machine, due to the risk. The LLM really can do anything under your user.

@czq99972
Copy link

All I had to do to use it is to set the runtime type to local in config.toml (e.g. like the template shows here.

But best use is probably not local exactly, but on another machine than your machine, due to the risk. The LLM really can do anything under your user.

Thank you, I will try to run it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enyst enyst enyst approved these changes

Assignees

@xingyaoww xingyaoww

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xingyaoww @openhands-agent @enyst @scosenza @jimwhite @czq99972